Skip to content

feat: create separate PRs per branch for downstream repos in cve.fix#101

Open
vmrh21 wants to merge 5 commits intoambient-code:mainfrom
angaduom:fix-branch-targeting-per-repo-type
Open

feat: create separate PRs per branch for downstream repos in cve.fix#101
vmrh21 wants to merge 5 commits intoambient-code:mainfrom
angaduom:fix-branch-targeting-per-repo-type

Conversation

@vmrh21
Copy link
Copy Markdown
Contributor

@vmrh21 vmrh21 commented Apr 2, 2026

Summary

Updates cve.fix to target branches differently based on repo_type:

  • upstream / midstream: PR against default_branch only (e.g. main)
  • downstream: separate PR for default_branch + each branch in active_release_branches

Each branch gets its own independent clone → fix → test → PR cycle. Branches are never combined into a single PR.

Example for red-hat-data-services/llm-d-inference-scheduler with active branches rhoai-3.3, rhoai-3.4, rhoai-3.4-ea.1, rhoai-3.4-ea.2:

  • PR 1 → base: main
  • PR 2 → base: rhoai-3.3
  • PR 3 → base: rhoai-3.4
  • PR 4 → base: rhoai-3.4-ea.1
  • PR 5 → base: rhoai-3.4-ea.2

No mapping file changes required — logic is derived from existing repo_type, default_branch, and active_release_branches fields.

Test plan

  • Run /cve.fix llm-d with an inference-scheduler CVE — verify upstream gets 1 PR (main only) and downstream gets 5 PRs (main + 4 release branches)
  • Verify each PR targets the correct base branch
  • Verify fix branch naming includes the target branch (e.g. fix/cve-YYYY-XXXXX-urllib3-rhoai-3.4-attempt-1)

🤖 Generated with Claude Code

- upstream/midstream: PR against default_branch only (main)
- downstream: separate PR for default_branch + each active_release_branch

Each branch is independently fixed and gets its own PR — never combined.
Fix branch naming includes the target branch for clarity, e.g.:
fix/cve-YYYY-XXXXX-<package>-rhoai-3.4-attempt-1

No mapping file changes needed — logic derived from existing
repo_type, default_branch, and active_release_branches fields.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 2, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

cve.fix now derives TARGET_BRANCHES from repo_type, authenticates via GitHub Classic PAT (gh), clones with depth=1, checks push permissions and falls back to forks, loops per target branch to apply fixes and open separate PRs, and reworks “CVE not found” to produce VEX artifacts. cve.find adds --ignore-vex and verifies Jira via /rest/api/3/myself.

Changes

Cohort / File(s) Summary
CVE fixer workflow
workflows/cve-fixer/.claude/commands/cve.fix.md
Changed branch targeting: upstream/midstreamdefault_branch; downstreamdefault_branch + active_release_branches. Switched to Classic PAT (gh auth login), clone with --depth=1 into /tmp/..., detect push rights via gh api, fallback to fork push and PR if no write access. Enforce per-branch loop: checkout/pull each target, apply fixes, commit, push, and open one PR per branch. Revised handling for “CVE not found”: perform Dockerfile base-image check (skopeo list-tags), update FROM if newer tag found (stop manifest fixes), otherwise add VEX justification artifacts: auto “Not Affected” (vex-justified-*) or human-review (vex-needs-human-review-*) with optional interactive prompt.
Jira access & search
workflows/cve-fixer/.claude/commands/cve.find.md
Added --ignore-vex flag and clarified --ignore-resolved wording. Replaced env-var presence checks with a /rest/api/3/myself Basic Auth call that branches on HTTP status (200 proceed; 401 prompt; 403 insufficient permissions; other → retry/timeout handling). Removed redundant AUTH re-assignment and reuse credentials from verification step.

Sequence Diagram(s)

sequenceDiagram
  participant WF as Workflow
  participant GH as GitHub CLI/API (gh)
  participant Git as Local Git
  participant Origin as Origin Repo
  participant Fork as Fork Repo
  participant Jira as Jira API

  WF->>GH: gh auth login (Classic PAT)
  WF->>Git: git clone --depth=1 /tmp/<org>/<repo>
  WF->>GH: gh api -> check can_push?
  alt has write access
    WF->>Git: for each TARGET_BRANCH: git checkout && git pull
    WF->>Git: apply fixes, commit, create branch
    Git->>Origin: git push branch
    WF->>GH: create PR to Origin
  else no write access
    WF->>Fork: push branch to Fork
    WF->>GH: create PR from Fork -> Origin
  end
  WF->>Jira: if CVE not found -> add VEX justification or create human-review artifact
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: enabling separate PRs per branch for downstream repos based on repo_type, which is the primary focus of the cve.fix workflow updates.
Description check ✅ Passed The description provides relevant context explaining the branch targeting logic, includes concrete examples, and outlines the test plan, all directly related to the changeset.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@workflows/cve-fixer/.claude/commands/cve.fix.md`:
- Around line 163-172: Update the fenced code blocks in
workflows/cve-fixer/.claude/commands/cve.fix.md to satisfy MD031/MD040 by adding
explicit language identifiers (e.g., ```text for the plain table and ```bash for
the shell block) and ensuring there is a blank line before and after each fenced
block; locate the blocks using the existing fence markers (``` and ```bash) and
the shown content ("upstream  llm-d/..." table and the "# Clone once:" shell
example) and insert the missing blank lines and the language tag on the opening
fence for each block.
- Around line 174-179: The branch naming guidance is inconsistent: the
multi-branch example uses a target-branch token (e.g.,
fix/cve-YYYY-XXXXX-<package>-rhoai-3.4-attempt-1) but the canonical rule later
omits it (fix/cve-...-<package>-attempt-1), risking collisions across base
branches; update the canonical naming rule to include a target-branch
placeholder (e.g., <target-branch> or the actual base branch name) and make both
the example and the canonical pattern consistent so every fix branch (symbols:
fix/cve-YYYY-XXXXX-<package>-<target-branch>-attempt-N) uniquely identifies the
base branch and prevents reuse/overwrite when repeating steps 4–11 for each
branch.
- Around line 183-191: The current flow that "Clone the repo once" then loops
over TARGET_BRANCHES (steps 5–11) can leak state between branch iterations;
update the procedure so each target branch runs in an isolated workspace: for
each branch in TARGET_BRANCHES either (a) create a fresh clone (git clone into a
new REPO_DIR per branch) or (b) perform a strict reset (git fetch --all; git
checkout <branch>; git reset --hard origin/<branch>; git clean -fdx) before
applying fixes, and ensure git credentials configuration (gh auth setup-git /
credential.helper / SSH fallback) is applied for each cloned workspace so
pushes/PRs cannot be affected by artifacts, uncommitted changes, or lockfile
drift from previous iterations.
- Around line 155-159: The TARGET_BRANCHES array construction can include
duplicates if ACTIVE_RELEASE_BRANCHES contains DEFAULT_BRANCH; change the logic
that sets TARGET_BRANCHES (the branch assembly around TARGET_BRANCHES,
DEFAULT_BRANCH and ACTIVE_RELEASE_BRANCHES) to deduplicate entries before use —
for example, build TARGET_BRANCHES by iterating DEFAULT_BRANCH and each element
of ACTIVE_RELEASE_BRANCHES and only appending branches not already present (or
use a temporary associative set) so TARGET_BRANCHES contains unique branch names
and avoids duplicate PR attempts.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: dcde3137-0bcb-4cf1-9ef6-55b5f3a67393

📥 Commits

Reviewing files that changed from the base of the PR and between 66bceaf and 96c3e63.

📒 Files selected for processing (1)
  • workflows/cve-fixer/.claude/commands/cve.fix.md

Comment on lines +155 to +159
if [ "$REPO_TYPE" = "downstream" ]; then
TARGET_BRANCHES=("$DEFAULT_BRANCH" "${ACTIVE_RELEASE_BRANCHES[@]}")
else
TARGET_BRANCHES=("$DEFAULT_BRANCH")
fi
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

TARGET_BRANCHES needs deduplication to avoid duplicate PR attempts.

If ACTIVE_RELEASE_BRANCHES contains DEFAULT_BRANCH, this creates duplicate targets and can trigger redundant PR creation/push conflicts.

Suggested doc fix
-  if [ "$REPO_TYPE" = "downstream" ]; then
-    TARGET_BRANCHES=("$DEFAULT_BRANCH" "${ACTIVE_RELEASE_BRANCHES[@]}")
-  else
-    TARGET_BRANCHES=("$DEFAULT_BRANCH")
-  fi
+  if [ "$REPO_TYPE" = "downstream" ]; then
+    TARGET_BRANCHES=("$DEFAULT_BRANCH" "${ACTIVE_RELEASE_BRANCHES[@]}")
+  else
+    TARGET_BRANCHES=("$DEFAULT_BRANCH")
+  fi
+
+  # Deduplicate while preserving order
+  UNIQUE_TARGET_BRANCHES=()
+  for b in "${TARGET_BRANCHES[@]}"; do
+    [[ -z "$b" ]] && continue
+    [[ " ${UNIQUE_TARGET_BRANCHES[*]} " == *" $b "* ]] || UNIQUE_TARGET_BRANCHES+=("$b")
+  done
+  TARGET_BRANCHES=("${UNIQUE_TARGET_BRANCHES[@]}")
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if [ "$REPO_TYPE" = "downstream" ]; then
TARGET_BRANCHES=("$DEFAULT_BRANCH" "${ACTIVE_RELEASE_BRANCHES[@]}")
else
TARGET_BRANCHES=("$DEFAULT_BRANCH")
fi
if [ "$REPO_TYPE" = "downstream" ]; then
TARGET_BRANCHES=("$DEFAULT_BRANCH" "${ACTIVE_RELEASE_BRANCHES[@]}")
else
TARGET_BRANCHES=("$DEFAULT_BRANCH")
fi
# Deduplicate while preserving order
UNIQUE_TARGET_BRANCHES=()
for b in "${TARGET_BRANCHES[@]}"; do
[[ -z "$b" ]] && continue
[[ " ${UNIQUE_TARGET_BRANCHES[*]} " == *" $b "* ]] || UNIQUE_TARGET_BRANCHES+=("$b")
done
TARGET_BRANCHES=("${UNIQUE_TARGET_BRANCHES[@]}")
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@workflows/cve-fixer/.claude/commands/cve.fix.md` around lines 155 - 159, The
TARGET_BRANCHES array construction can include duplicates if
ACTIVE_RELEASE_BRANCHES contains DEFAULT_BRANCH; change the logic that sets
TARGET_BRANCHES (the branch assembly around TARGET_BRANCHES, DEFAULT_BRANCH and
ACTIVE_RELEASE_BRANCHES) to deduplicate entries before use — for example, build
TARGET_BRANCHES by iterating DEFAULT_BRANCH and each element of
ACTIVE_RELEASE_BRANCHES and only appending branches not already present (or use
a temporary associative set) so TARGET_BRANCHES contains unique branch names and
avoids duplicate PR attempts.

Comment on lines +174 to 179
**Multi-repo + multi-branch strategy**:
- Fix upstream repos first, then midstream, then downstream
- For downstream: Steps 4 through 11 repeat for EACH branch independently
- Each branch produces its own PR with its own fix branch (e.g., `fix/cve-YYYY-XXXXX-<package>-rhoai-3.4-attempt-1`)
- Never combine fixes for multiple branches into a single PR

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Branch naming rule is still inconsistent and can collide across target branches.

This section says branch names include the target branch, but the canonical naming rule later still omits it (fix/cve-...-<package>-attempt-1). That can overwrite/reuse the same fix branch name for multiple base branches.

Suggested doc fix
-  - Use consistent naming: `fix/cve-YYYY-XXXXX-<package-name>-attempt-1`
+  - Use consistent naming: `fix/cve-YYYY-XXXXX-<package-name>-<target-branch>-attempt-1`
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@workflows/cve-fixer/.claude/commands/cve.fix.md` around lines 174 - 179, The
branch naming guidance is inconsistent: the multi-branch example uses a
target-branch token (e.g., fix/cve-YYYY-XXXXX-<package>-rhoai-3.4-attempt-1) but
the canonical rule later omits it (fix/cve-...-<package>-attempt-1), risking
collisions across base branches; update the canonical naming rule to include a
target-branch placeholder (e.g., <target-branch> or the actual base branch name)
and make both the example and the canonical pattern consistent so every fix
branch (symbols: fix/cve-YYYY-XXXXX-<package>-<target-branch>-attempt-N)
uniquely identifies the base branch and prevents reuse/overwrite when repeating
steps 4–11 for each branch.

Ambient sessions inject secrets differently — JIRA_API_TOKEN and
JIRA_EMAIL may be available to API calls even when not visible to
bash [ -z "$VAR" ] checks, causing false "missing credentials" errors.

Replace the upfront env var check with a lightweight GET /myself test
call: proceed on 200, only complain on 401/403. Never hard-stop just
because the shell check returns empty.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@workflows/cve-fixer/.claude/commands/cve.find.md`:
- Around line 83-87: Insert a single blank line before the fenced code block in
the "a. Set up variables (AUTH already set from Step 2):" list item so the
markdown has a blank line immediately before the ```bash fence (fixing MD031).
Locate the fenced block in workflows/cve-fixer/.claude/commands/cve.find.md
under the "COMPONENT_NAME" setup section and add the blank line above the
```bash line to make the block compliant.
- Around line 69-75: The fenced code block starting with "```bash" containing
the export lines currently lacks surrounding blank lines (MD031); add one blank
line immediately before the opening "```bash" and one blank line immediately
after the closing "```" so the code block is separated from the surrounding list
text (update the block in workflows/cve-fixer/.claude/commands/cve.find.md where
the export JIRA_API_TOKEN/JIRA_EMAIL lines appear).
- Around line 58-79: The curl healthcheck using JIRA_BASE_URL and TEST_RESPONSE
should not treat HTTP 403 as "insufficient permissions" because
/rest/api/3/myself returns 401 for both auth failures and permission issues;
update the logic that branches on TEST_RESPONSE to consolidate 401 handling (log
both missing/invalid creds and insufficient permissions together and instruct
the user to check API token/email and permissions) and remove or disable the
separate 403 branch, or alternatively replace the endpoint with a
permissions-specific endpoint if you really need to distinguish 403; keep the
existing note about not pre-checking env vars (do not add a [ -z
"$JIRA_API_TOKEN" ] check).
- Around line 58-77: The documentation says to "retry once" on network/timeout
but the TEST_RESPONSE curl call lacks retry options; update the curl invocation
that sets TEST_RESPONSE (the command using JIRA_BASE_URL and AUTH) to perform a
single retry on transient failures by adding curl retry flags (e.g., --retry 1
plus an appropriate --retry-delay and --retry-connrefused) while preserving
existing timeouts (--connect-timeout, --max-time) and the exit-code capture (-s
-o /dev/null -w "%{http_code}"); ensure the adjusted curl still returns an HTTP
status code into TEST_RESPONSE so the subsequent HTTP 200/401/403/Other handling
continues to work.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 2f7d15b0-6ed9-419e-b03a-15674de9f5c6

📥 Commits

Reviewing files that changed from the base of the PR and between 96c3e63 and 340b108.

📒 Files selected for processing (1)
  • workflows/cve-fixer/.claude/commands/cve.find.md

Comment on lines +58 to +79
```bash
JIRA_BASE_URL="https://redhat.atlassian.net"
AUTH=$(echo -n "${JIRA_EMAIL}:${JIRA_API_TOKEN}" | base64)
TEST_RESPONSE=$(curl -s -o /dev/null -w "%{http_code}" -X GET \
--connect-timeout 10 --max-time 15 \
-H "Authorization: Basic ${AUTH}" \
-H "Content-Type: application/json" \
"${JIRA_BASE_URL}/rest/api/3/myself")
```

- **HTTP 200** → credentials valid, proceed
- **HTTP 401** → credentials missing or invalid. Only now inform the user:
- Check if `JIRA_API_TOKEN` and `JIRA_EMAIL` are configured as Ambient session secrets
- If not, generate a token at https://id.atlassian.com/manage-profile/security/api-tokens and export:
```bash
export JIRA_API_TOKEN="your-token-here"
export JIRA_EMAIL="your-email@redhat.com"
```
- **HTTP 403** → token valid but insufficient permissions — inform user
- **Other / timeout** → network issue — inform user and retry once

**Do NOT pre-check env vars with `[ -z "$JIRA_API_TOKEN" ]` and stop.** The variables may be available to the API call even if not visible to the shell check (e.g. Ambient secrets injection).
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🌐 Web query:

What is the recommended Jira REST API v3 endpoint for testing authentication credentials, and what HTTP status codes does it return for valid credentials, invalid credentials, and insufficient permissions?

💡 Result:

The recommended Jira REST API v3 endpoint for testing authentication credentials is GET /rest/api/3/myself. This endpoint returns details for the current authenticated user and requires only "Permission to access Jira". HTTP status codes: - Valid credentials: 200 OK - Invalid credentials: 401 Unauthorized - Insufficient permissions: This endpoint has minimal permissions required (access Jira), so insufficient permissions would also return 401 Unauthorized if the user lacks basic access. Alternative endpoint for permission checks: GET /rest/api/3/mypermissions, which returns the user's permissions and can be used to verify more granular access. It returns 200 OK for valid authenticated users. Note: There is no dedicated "health check" or ping endpoint solely for auth testing in official docs; /myself is commonly used as it confirms successful authentication and basic access.

Citations:


Reconsider the HTTP 403 status code—/rest/api/3/myself returns 401 for all authentication failures.

The endpoint is correct for credentials validation. However, the /rest/api/3/myself endpoint returns HTTP 401 for both missing/invalid credentials and insufficient permissions. The current code handles HTTP 403 as a separate "insufficient permissions" case, but this endpoint does not return 403 per official Jira REST API v3 documentation.

Consolidate error handling to treat 401 responses uniformly, or verify that a different endpoint is intended for the 403 case.

🧰 Tools
🪛 markdownlint-cli2 (0.22.0)

[warning] 72-72: Fenced code blocks should be surrounded by blank lines

(MD031, blanks-around-fences)


[warning] 75-75: Fenced code blocks should be surrounded by blank lines

(MD031, blanks-around-fences)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@workflows/cve-fixer/.claude/commands/cve.find.md` around lines 58 - 79, The
curl healthcheck using JIRA_BASE_URL and TEST_RESPONSE should not treat HTTP 403
as "insufficient permissions" because /rest/api/3/myself returns 401 for both
auth failures and permission issues; update the logic that branches on
TEST_RESPONSE to consolidate 401 handling (log both missing/invalid creds and
insufficient permissions together and instruct the user to check API token/email
and permissions) and remove or disable the separate 403 branch, or alternatively
replace the endpoint with a permissions-specific endpoint if you really need to
distinguish 403; keep the existing note about not pre-checking env vars (do not
add a [ -z "$JIRA_API_TOKEN" ] check).

Comment on lines +58 to +77
```bash
JIRA_BASE_URL="https://redhat.atlassian.net"
AUTH=$(echo -n "${JIRA_EMAIL}:${JIRA_API_TOKEN}" | base64)
TEST_RESPONSE=$(curl -s -o /dev/null -w "%{http_code}" -X GET \
--connect-timeout 10 --max-time 15 \
-H "Authorization: Basic ${AUTH}" \
-H "Content-Type: application/json" \
"${JIRA_BASE_URL}/rest/api/3/myself")
```

- **HTTP 200** → credentials valid, proceed
- **HTTP 401** → credentials missing or invalid. Only now inform the user:
- Check if `JIRA_API_TOKEN` and `JIRA_EMAIL` are configured as Ambient session secrets
- If not, generate a token at https://id.atlassian.com/manage-profile/security/api-tokens and export:
```bash
export JIRA_API_TOKEN="your-token-here"
export JIRA_EMAIL="your-email@redhat.com"
```
- **HTTP 403** → token valid but insufficient permissions — inform user
- **Other / timeout** → network issue — inform user and retry once
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Implement retry logic for timeout handling.

Line 77 documents that timeout/network errors should "retry once," but the test API call in lines 58-66 lacks retry flags. This creates a documentation-implementation gap.

🔄 Proposed fix to add retry logic
 TEST_RESPONSE=$(curl -s -o /dev/null -w "%{http_code}" -X GET \
   --connect-timeout 10 --max-time 15 \
+  --retry 1 \
+  --retry-delay 2 \
+  --retry-connrefused \
   -H "Authorization: Basic ${AUTH}" \
   -H "Content-Type: application/json" \
   "${JIRA_BASE_URL}/rest/api/3/myself")
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)

[warning] 72-72: Fenced code blocks should be surrounded by blank lines

(MD031, blanks-around-fences)


[warning] 75-75: Fenced code blocks should be surrounded by blank lines

(MD031, blanks-around-fences)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@workflows/cve-fixer/.claude/commands/cve.find.md` around lines 58 - 77, The
documentation says to "retry once" on network/timeout but the TEST_RESPONSE curl
call lacks retry options; update the curl invocation that sets TEST_RESPONSE
(the command using JIRA_BASE_URL and AUTH) to perform a single retry on
transient failures by adding curl retry flags (e.g., --retry 1 plus an
appropriate --retry-delay and --retry-connrefused) while preserving existing
timeouts (--connect-timeout, --max-time) and the exit-code capture (-s -o
/dev/null -w "%{http_code}"); ensure the adjusted curl still returns an HTTP
status code into TEST_RESPONSE so the subsequent HTTP 200/401/403/Other handling
continues to work.

Comment on lines +69 to +75
- **HTTP 401** → credentials missing or invalid. Only now inform the user:
- Check if `JIRA_API_TOKEN` and `JIRA_EMAIL` are configured as Ambient session secrets
- If not, generate a token at https://id.atlassian.com/manage-profile/security/api-tokens and export:
```bash
export JIRA_API_TOKEN="your-token-here"
export JIRA_EMAIL="your-email@redhat.com"
```
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Add blank lines around fenced code blocks for markdown compliance.

Markdownlint reports missing blank lines around the code block per MD031. Add a blank line before line 72 and after line 75 to improve formatting consistency.

📝 Proposed formatting fix
   - **HTTP 401** → credentials missing or invalid. Only now inform the user:
     - Check if `JIRA_API_TOKEN` and `JIRA_EMAIL` are configured as Ambient session secrets
     - If not, generate a token at https://id.atlassian.com/manage-profile/security/api-tokens and export:
+
       ```bash
       export JIRA_API_TOKEN="your-token-here"
       export JIRA_EMAIL="your-email@redhat.com"
       ```
+
   - **HTTP 403** → token valid but insufficient permissions — inform user
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)

[warning] 72-72: Fenced code blocks should be surrounded by blank lines

(MD031, blanks-around-fences)


[warning] 75-75: Fenced code blocks should be surrounded by blank lines

(MD031, blanks-around-fences)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@workflows/cve-fixer/.claude/commands/cve.find.md` around lines 69 - 75, The
fenced code block starting with "```bash" containing the export lines currently
lacks surrounding blank lines (MD031); add one blank line immediately before the
opening "```bash" and one blank line immediately after the closing "```" so the
code block is separated from the surrounding list text (update the block in
workflows/cve-fixer/.claude/commands/cve.find.md where the export
JIRA_API_TOKEN/JIRA_EMAIL lines appear).

Comment on lines +83 to +87
a. Set up variables (AUTH already set from Step 2):
```bash
COMPONENT_NAME="[from step 1]"
JIRA_BASE_URL="https://redhat.atlassian.net"
JIRA_EMAIL="${JIRA_EMAIL}"
JIRA_API_TOKEN="${JIRA_API_TOKEN}"
# Jira Cloud uses Basic Auth: base64(email:api-token)
AUTH=$(echo -n "${JIRA_EMAIL}:${JIRA_API_TOKEN}" | base64)
# AUTH already constructed in Step 2 — reuse it
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Add blank line before fenced code block for markdown compliance.

Markdownlint reports a missing blank line before the code block at line 84 per MD031.

📝 Proposed formatting fix
 3. **Query Jira for CVE Issues**
 
    a. Set up variables (AUTH already set from Step 2):
+
    ```bash
    COMPONENT_NAME="[from step 1]"
    JIRA_BASE_URL="https://redhat.atlassian.net"
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)

[warning] 84-84: Fenced code blocks should be surrounded by blank lines

(MD031, blanks-around-fences)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@workflows/cve-fixer/.claude/commands/cve.find.md` around lines 83 - 87,
Insert a single blank line before the fenced code block in the "a. Set up
variables (AUTH already set from Step 2):" list item so the markdown has a blank
line immediately before the ```bash fence (fixing MD031). Locate the fenced
block in workflows/cve-fixer/.claude/commands/cve.find.md under the
"COMPONENT_NAME" setup section and add the blank line above the ```bash line to
make the block compliant.

…access repos

- Recommend Classic PAT with repo scope over fine-grained PAT or
  Ambient Code GitHub App (neither works reliably for cross-org repos
  like llm-d/*, eval-hub/*, trustyai-explainability/*)
- Add write access check: use gh api repos/X --jq '.permissions.push'
  before attempting to push
- Add fork fallback: if no write access, create fork, push fix branch
  to fork, and open PR targeting the original repo with head: user:branch
  This handles all upstream external org repos automatically

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (4)
workflows/cve-fixer/.claude/commands/cve.fix.md (4)

153-160: ⚠️ Potential issue | 🟠 Major

Deduplicate TARGET_BRANCHES before processing to prevent duplicate PR attempts.

If ACTIVE_RELEASE_BRANCHES contains DEFAULT_BRANCH, this list can duplicate targets and cause redundant PR/push attempts.

Suggested doc fix
 if [ "$REPO_TYPE" = "downstream" ]; then
   TARGET_BRANCHES=("$DEFAULT_BRANCH" "${ACTIVE_RELEASE_BRANCHES[@]}")
 else
   TARGET_BRANCHES=("$DEFAULT_BRANCH")
 fi
+
+# Deduplicate while preserving order
+UNIQUE_TARGET_BRANCHES=()
+for b in "${TARGET_BRANCHES[@]}"; do
+  [ -z "$b" ] && continue
+  [[ " ${UNIQUE_TARGET_BRANCHES[*]} " == *" $b "* ]] || UNIQUE_TARGET_BRANCHES+=("$b")
+done
+TARGET_BRANCHES=("${UNIQUE_TARGET_BRANCHES[@]}")
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@workflows/cve-fixer/.claude/commands/cve.fix.md` around lines 153 - 160, The
TARGET_BRANCHES assignment can produce duplicates when ACTIVE_RELEASE_BRANCHES
includes DEFAULT_BRANCH; update the logic that builds TARGET_BRANCHES (the block
using REPO_TYPE, DEFAULT_BRANCH and ACTIVE_RELEASE_BRANCHES) to deduplicate
entries before further processing — for example, after composing
TARGET_BRANCHES, iterate and rebuild a new array using a seen set (declare -A
seen) to skip already-added branch names so each branch appears only once;
ensure downstream consumers of TARGET_BRANCHES use the deduplicated array.

162-173: ⚠️ Potential issue | 🟡 Minor

Fix fenced code block formatting for markdownlint compliance (MD031/MD040).

This fence is missing a language identifier and blank-line framing.

Suggested doc fix
-   ```
+   
+   ```text
    upstream  llm-d/llm-d-inference-scheduler          → PR against: main
    midstream opendatahub-io/llm-d-inference-scheduler  → PR against: main
    downstream red-hat-data-services/llm-d-inference-scheduler → PRs against:
      - main
      - rhoai-3.3
      - rhoai-3.4
      - rhoai-3.4-ea.1
      - rhoai-3.4-ea.2
    ```
+
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@workflows/cve-fixer/.claude/commands/cve.fix.md` around lines 162 - 173, The
fenced code block for the "llm-d inference-scheduler" example is missing a
language identifier and blank-line framing; update the markdown by adding a
blank line before and after the triple-backtick fence and include a language
token (e.g., "text") after the opening ``` so the block around the "upstream
llm-d/llm-d-inference-scheduler ..." example is fenced as ```text ... ``` to
satisfy MD031/MD040.

206-207: ⚠️ Potential issue | 🟠 Major

Single-clone branch loop still contradicts per-target isolated execution.

Current guidance still uses one clone + checkout loop, which risks state leakage across target branches and conflicts with the PR objective of independent clone→fix→test→PR per branch.

Suggested doc fix
-REPO_DIR="/tmp/${REPO_ORG}/${REPO_NAME}"
+REPO_DIR_BASE="/tmp/${REPO_ORG}"
...
-Steps 5–11 run in a loop over `TARGET_BRANCHES` — for each branch:
-- `git checkout <branch>` and `git pull` to ensure it is up to date
-- Apply fix and create PR targeting that branch (from fork if no direct push access)
+Steps 5–11 run independently per `TARGET_BRANCH` — for each branch:
+- Create branch-specific workspace: `REPO_DIR="${REPO_DIR_BASE}/${REPO_NAME}-${TARGET_BRANCH}"`
+- Clone/fetch in that workspace
+- Checkout the target branch in that workspace
+- Apply fix, run tests, and create PR for that target branch only

Also applies to: 239-243, 246-258

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@workflows/cve-fixer/.claude/commands/cve.fix.md` around lines 206 - 207, The
workflow currently sets a single REPO_DIR and then iterates branches using
checkout loops, which causes state leakage; modify the flow so each target
branch performs an independent clone into its own temporary directory (create a
fresh REPO_DIR per-target inside the per-target loop), perform git checkout, run
the fix/test/PR sequence, and then clean up that temp clone; update mentions of
REPO_DIR, the per-target loop, and any checkout logic to reflect per-target
isolated clone→checkout→fix→test→PR execution and ensure cleanup after each
target.

174-178: ⚠️ Potential issue | 🟠 Major

Branch naming guidance is still inconsistent with canonical naming section.

These lines correctly include <target-branch> in examples, but the canonical pattern later still omits it (fix/cve-...-<package>-attempt-1), which can collide across bases.

Suggested doc fix
-- Use consistent naming: `fix/cve-YYYY-XXXXX-<package-name>-attempt-1`
+- Use consistent naming: `fix/cve-YYYY-XXXXX-<package-name>-<target-branch>-attempt-1`
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@workflows/cve-fixer/.claude/commands/cve.fix.md` around lines 174 - 178,
Update the canonical branch-naming pattern to include the <target-branch> token
so it matches the earlier examples; specifically change the canonical example
`fix/cve-...-<package>-attempt-1` to the consistent pattern used above
`fix/cve-YYYY-XXXXX-<package>-<target-branch>-attempt-1` (or equivalent ordering
used in the Multi-repo + multi-branch examples) and update any other occurrences
in the document so all examples and the canonical section use the same
`<target-branch>`-inclusive format to avoid cross-branch collisions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@workflows/cve-fixer/.claude/commands/cve.fix.md`:
- Around line 153-160: The TARGET_BRANCHES assignment can produce duplicates
when ACTIVE_RELEASE_BRANCHES includes DEFAULT_BRANCH; update the logic that
builds TARGET_BRANCHES (the block using REPO_TYPE, DEFAULT_BRANCH and
ACTIVE_RELEASE_BRANCHES) to deduplicate entries before further processing — for
example, after composing TARGET_BRANCHES, iterate and rebuild a new array using
a seen set (declare -A seen) to skip already-added branch names so each branch
appears only once; ensure downstream consumers of TARGET_BRANCHES use the
deduplicated array.
- Around line 162-173: The fenced code block for the "llm-d inference-scheduler"
example is missing a language identifier and blank-line framing; update the
markdown by adding a blank line before and after the triple-backtick fence and
include a language token (e.g., "text") after the opening ``` so the block
around the "upstream llm-d/llm-d-inference-scheduler ..." example is fenced as
```text ... ``` to satisfy MD031/MD040.
- Around line 206-207: The workflow currently sets a single REPO_DIR and then
iterates branches using checkout loops, which causes state leakage; modify the
flow so each target branch performs an independent clone into its own temporary
directory (create a fresh REPO_DIR per-target inside the per-target loop),
perform git checkout, run the fix/test/PR sequence, and then clean up that temp
clone; update mentions of REPO_DIR, the per-target loop, and any checkout logic
to reflect per-target isolated clone→checkout→fix→test→PR execution and ensure
cleanup after each target.
- Around line 174-178: Update the canonical branch-naming pattern to include the
<target-branch> token so it matches the earlier examples; specifically change
the canonical example `fix/cve-...-<package>-attempt-1` to the consistent
pattern used above `fix/cve-YYYY-XXXXX-<package>-<target-branch>-attempt-1` (or
equivalent ordering used in the Multi-repo + multi-branch examples) and update
any other occurrences in the document so all examples and the canonical section
use the same `<target-branch>`-inclusive format to avoid cross-branch
collisions.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: c2120c28-59ea-4e75-8e7d-6748971ae08e

📥 Commits

Reviewing files that changed from the base of the PR and between 340b108 and 177e6d0.

📒 Files selected for processing (1)
  • workflows/cve-fixer/.claude/commands/cve.fix.md

cve.fix: when both scan and package check find no CVE evidence:
- Auto-determine VEX justification where possible:
  1. Component not Present — package not in any manifest (auto)
  2. Vulnerable Code not Present — package at safe version (auto)
  3. Vulnerable Code not in Execute Path — govulncheck call graph (auto, Go only)
  4. Vulnerable Code cannot be Controlled by Adversary — human judgment
  5. Inline Mitigations already Exist — human judgment
- For cases 1-3: add Jira comment with justification + evidence, do not auto-close
- For cases 4-5: log in artifacts/vex-needs-human-review-CVE-*.md, skip Jira comment
- In interactive mode: prompt user to select from all 5 options

cve.find: add --ignore-vex flag to exclude issues already closed with
VEX justification (Not a Bug / Obsolete / Won't Fix)

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
workflows/cve-fixer/.claude/commands/cve.find.md (1)

36-46: ⚠️ Potential issue | 🟠 Major

Persist --ignore-vex as a boolean alongside --ignore-resolved.

Step 1 introduces --ignore-vex, but the storage note still only documents --ignore-resolved. That leaves Step 3’s IGNORE_VEX branch under-specified.

Suggested doc patch
-   - Store the `--ignore-resolved` flag as a boolean for use in step 3
+   - Store `--ignore-resolved` and `--ignore-vex` flags as booleans for use in step 3
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@workflows/cve-fixer/.claude/commands/cve.find.md` around lines 36 - 46, The
docs mention the --ignore-vex flag but never instruct storing it; update the
behavior notes so --ignore-vex is persisted as a boolean just like
--ignore-resolved (e.g., document IGNORE_VEX alongside IGNORE_RESOLVED), and
clarify Step 3’s IGNORE_VEX branch to read that boolean when filtering results;
ensure the instruction still forbids using AskUserQuestion or presenting options
and that component prompting remains unchanged.
♻️ Duplicate comments (6)
workflows/cve-fixer/.claude/commands/cve.find.md (2)

72-76: ⚠️ Potential issue | 🟡 Minor

Fix MD031 fenced-block spacing in changed list items.

The fenced bash blocks here still need blank lines before/after to satisfy markdownlint.

Also applies to: 84-89

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@workflows/cve-fixer/.claude/commands/cve.find.md` around lines 72 - 76, Add a
blank line before and after each fenced bash code block that contains the export
lines (the block starting with the URL
"https://id.atlassian.com/manage-profile/security/api-tokens" and the block with
"export JIRA_API_TOKEN" / "export JIRA_EMAIL") so the fenced blocks have an
empty line above and below to satisfy MD031; update both occurrences (the block
containing the export lines and the later duplicate block) accordingly.

62-78: ⚠️ Potential issue | 🟠 Major

Auth probe logic for /myself is still inconsistent and retry is undocumented in command behavior.

This block still treats HTTP 403 separately and says “retry once” without curl retry flags. Either consolidate 401 handling for /myself, or switch endpoint if you need permission-specific branching.

Suggested doc patch
 TEST_RESPONSE=$(curl -s -o /dev/null -w "%{http_code}" -X GET \
   --connect-timeout 10 --max-time 15 \
+  --retry 1 --retry-delay 2 --retry-connrefused \
   -H "Authorization: Basic ${AUTH}" \
   -H "Content-Type: application/json" \
   "${JIRA_BASE_URL}/rest/api/3/myself")
@@
-  - **HTTP 403** → token valid but insufficient permissions — inform user
-  - **Other / timeout** → network issue — inform user and retry once
+  - **Other / timeout** → network issue — inform user (probe includes one retry)
For Jira Cloud REST API v3, what are the documented response codes for GET /rest/api/3/myself, specifically whether 403 is expected versus 401 for auth/access failures?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@workflows/cve-fixer/.claude/commands/cve.find.md` around lines 62 - 78, The
auth probe using GET "${JIRA_BASE_URL}/rest/api/3/myself" is inconsistent: it
documents a special HTTP 403 case and “retry once” but the curl invocation
(TEST_RESPONSE) has no retry behavior and the endpoint may not reliably
differentiate 401 vs 403 for auth issues; update the logic around TEST_RESPONSE
to either consolidate 401/403 handling for auth failures or switch to a
permission-specific endpoint if you need to detect permission vs credential
problems, and explicitly implement a single retry (using curl retry flags or a
coded retry loop) when handling network/timeout responses so the documented
“retry once” matches actual behavior; ensure messages reference TEST_RESPONSE
and the "/rest/api/3/myself" probe so maintainers can find and update the probe
logic.
workflows/cve-fixer/.claude/commands/cve.fix.md (4)

239-258: ⚠️ Potential issue | 🟠 Major

“Clone once + branch loop” conflicts with independent branch execution.

This section still describes shared workspace iteration, which can leak state between branch runs and contradicts the per-target independent cycle.

Suggested doc patch
- Steps 5–11 run in a loop over `TARGET_BRANCHES` — for each branch:
- - `git checkout <branch>` and `git pull` to ensure it is up to date
- - Apply fix and create PR targeting that branch (from fork if no direct push access)
+ Steps 5–11 run independently per `TARGET_BRANCH` in isolated workspaces:
+ - Create branch-specific clone dir (e.g., `/tmp/${REPO_ORG}/${REPO_NAME}-${TARGET_BRANCH}`)
+ - Checkout target branch in that clone, apply fix, test, and create PR
+ - Do not reuse working tree state across target branches
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@workflows/cve-fixer/.claude/commands/cve.fix.md` around lines 239 - 258, The
documentation section "4.3: Branch loop" currently describes a single cloned
workspace iterating over TARGET_BRANCHES and may leak state; update the text to
reflect the intended independent-per-branch execution model by removing or
clarifying the "Clone once" example and the shared workspace loop (steps 5–11)
and instead describe that each TARGET_BRANCH run should perform its own
checkout/pull/apply-create-PR cycle in an isolated workspace (or explicitly
document the alternative "clone once" mode as a separate, stateful option),
referencing the TARGET_BRANCHES loop and the checkout/pull/apply PR steps to
ensure readers know to avoid shared-state side effects between branch runs.

174-178: ⚠️ Potential issue | 🟠 Major

Make branch naming rule consistent with multi-branch behavior.

This section requires branch-qualified fix names, but the canonical rule later (Line 573) still omits <target-branch>, which can collide across base branches.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@workflows/cve-fixer/.claude/commands/cve.fix.md` around lines 174 - 178, The
branch naming guidance is inconsistent: the multi-branch section requires
branch-qualified names (example
`fix/cve-YYYY-XXXXX-<package>-rhoai-3.4-attempt-1`) but the canonical naming
rule later omits the `<target-branch>` component, risking name collisions;
update the canonical rule to include `<target-branch>` (or a `<base-branch>`
token) so it matches the multi-branch example and ensure all references to the
canonical rule and examples use the same format including the branch qualifier.

153-160: ⚠️ Potential issue | 🟠 Major

Deduplicate TARGET_BRANCHES to prevent duplicate PR attempts.

If ACTIVE_RELEASE_BRANCHES contains DEFAULT_BRANCH, this creates duplicate targets.

Suggested doc patch
 if [ "$REPO_TYPE" = "downstream" ]; then
   TARGET_BRANCHES=("$DEFAULT_BRANCH" "${ACTIVE_RELEASE_BRANCHES[@]}")
 else
   TARGET_BRANCHES=("$DEFAULT_BRANCH")
 fi
+
+# Deduplicate while preserving order
+UNIQUE_TARGET_BRANCHES=()
+for b in "${TARGET_BRANCHES[@]}"; do
+  [ -z "$b" ] && continue
+  [[ " ${UNIQUE_TARGET_BRANCHES[*]} " == *" $b "* ]] || UNIQUE_TARGET_BRANCHES+=("$b")
+done
+TARGET_BRANCHES=("${UNIQUE_TARGET_BRANCHES[@]}")
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@workflows/cve-fixer/.claude/commands/cve.fix.md` around lines 153 - 160, The
TARGET_BRANCHES assignment can yield duplicates when ACTIVE_RELEASE_BRANCHES
contains DEFAULT_BRANCH; update the logic that builds TARGET_BRANCHES (the
branch selection block using TARGET_BRANCHES, DEFAULT_BRANCH, and
ACTIVE_RELEASE_BRANCHES) to deduplicate entries before using them — for example,
after composing the array, filter it to unique values (using an associative
"seen" map loop or a stdout-unique transform like sort -u/awk) and reassign the
cleaned list back to TARGET_BRANCHES so no duplicate PR targets are attempted.

163-172: ⚠️ Potential issue | 🟡 Minor

Fix markdown fences (MD031/MD040) in changed examples.

These blocks still need language tags and blank-line framing around fenced blocks.

Also applies to: 473-481

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@workflows/cve-fixer/.claude/commands/cve.fix.md` around lines 163 - 172, The
fenced code examples lack language tags and surrounding blank lines, which
triggers MD031/MD040; update each triple-backtick block (e.g., the block
starting with "upstream  llm-d/llm-d-inference-scheduler          → PR against:
main" and the other occurrence around the content referenced as 473-481) to add
an appropriate language tag (like ```text or ```bash) immediately after the
opening backticks and ensure there is a blank line before and after the fenced
block so the markdown linter accepts the examples; apply this change
consistently to all similar fenced blocks in the document.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@workflows/cve-fixer/.claude/commands/cve.fix.md`:
- Around line 439-443: Update the output/reporting docs to reference the new VEX
artifact names and flows instead of the old "already-fixed" wording: replace
references to `artifacts/cve-fixer/fixes/vex-justified-CVE-YYYY-XXXXX.md` and
any mentions of the old “already-fixed” flow with the corresponding new artifact
names (`vex-justified-*`, `vex-needs-human-review-*`) and update the
printed/report messages (e.g., the "✅ CVE-YYYY-XXXXX not present..." line) and
the bullet list for the auto-determined justification cases to explicitly
document writing the correct VEX artifact and leaving closure to a human
reviewer.

---

Outside diff comments:
In `@workflows/cve-fixer/.claude/commands/cve.find.md`:
- Around line 36-46: The docs mention the --ignore-vex flag but never instruct
storing it; update the behavior notes so --ignore-vex is persisted as a boolean
just like --ignore-resolved (e.g., document IGNORE_VEX alongside
IGNORE_RESOLVED), and clarify Step 3’s IGNORE_VEX branch to read that boolean
when filtering results; ensure the instruction still forbids using
AskUserQuestion or presenting options and that component prompting remains
unchanged.

---

Duplicate comments:
In `@workflows/cve-fixer/.claude/commands/cve.find.md`:
- Around line 72-76: Add a blank line before and after each fenced bash code
block that contains the export lines (the block starting with the URL
"https://id.atlassian.com/manage-profile/security/api-tokens" and the block with
"export JIRA_API_TOKEN" / "export JIRA_EMAIL") so the fenced blocks have an
empty line above and below to satisfy MD031; update both occurrences (the block
containing the export lines and the later duplicate block) accordingly.
- Around line 62-78: The auth probe using GET
"${JIRA_BASE_URL}/rest/api/3/myself" is inconsistent: it documents a special
HTTP 403 case and “retry once” but the curl invocation (TEST_RESPONSE) has no
retry behavior and the endpoint may not reliably differentiate 401 vs 403 for
auth issues; update the logic around TEST_RESPONSE to either consolidate 401/403
handling for auth failures or switch to a permission-specific endpoint if you
need to detect permission vs credential problems, and explicitly implement a
single retry (using curl retry flags or a coded retry loop) when handling
network/timeout responses so the documented “retry once” matches actual
behavior; ensure messages reference TEST_RESPONSE and the "/rest/api/3/myself"
probe so maintainers can find and update the probe logic.

In `@workflows/cve-fixer/.claude/commands/cve.fix.md`:
- Around line 239-258: The documentation section "4.3: Branch loop" currently
describes a single cloned workspace iterating over TARGET_BRANCHES and may leak
state; update the text to reflect the intended independent-per-branch execution
model by removing or clarifying the "Clone once" example and the shared
workspace loop (steps 5–11) and instead describe that each TARGET_BRANCH run
should perform its own checkout/pull/apply-create-PR cycle in an isolated
workspace (or explicitly document the alternative "clone once" mode as a
separate, stateful option), referencing the TARGET_BRANCHES loop and the
checkout/pull/apply PR steps to ensure readers know to avoid shared-state side
effects between branch runs.
- Around line 174-178: The branch naming guidance is inconsistent: the
multi-branch section requires branch-qualified names (example
`fix/cve-YYYY-XXXXX-<package>-rhoai-3.4-attempt-1`) but the canonical naming
rule later omits the `<target-branch>` component, risking name collisions;
update the canonical rule to include `<target-branch>` (or a `<base-branch>`
token) so it matches the multi-branch example and ensure all references to the
canonical rule and examples use the same format including the branch qualifier.
- Around line 153-160: The TARGET_BRANCHES assignment can yield duplicates when
ACTIVE_RELEASE_BRANCHES contains DEFAULT_BRANCH; update the logic that builds
TARGET_BRANCHES (the branch selection block using TARGET_BRANCHES,
DEFAULT_BRANCH, and ACTIVE_RELEASE_BRANCHES) to deduplicate entries before using
them — for example, after composing the array, filter it to unique values (using
an associative "seen" map loop or a stdout-unique transform like sort -u/awk)
and reassign the cleaned list back to TARGET_BRANCHES so no duplicate PR targets
are attempted.
- Around line 163-172: The fenced code examples lack language tags and
surrounding blank lines, which triggers MD031/MD040; update each triple-backtick
block (e.g., the block starting with "upstream  llm-d/llm-d-inference-scheduler 
→ PR against: main" and the other occurrence around the content referenced as
473-481) to add an appropriate language tag (like ```text or ```bash)
immediately after the opening backticks and ensure there is a blank line before
and after the fenced block so the markdown linter accepts the examples; apply
this change consistently to all similar fenced blocks in the document.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 4dda33f7-a5a1-4ee0-a166-ae7ab194816a

📥 Commits

Reviewing files that changed from the base of the PR and between 177e6d0 and 841df52.

📒 Files selected for processing (2)
  • workflows/cve-fixer/.claude/commands/cve.find.md
  • workflows/cve-fixer/.claude/commands/cve.fix.md

Comment on lines +439 to +443
**If justification auto-determined (cases 1, 2, 3):**
- Add a comment to the Jira issue with the justification and evidence
- Do NOT auto-close the issue — leave closing to the human reviewer
- Document in `artifacts/cve-fixer/fixes/vex-justified-CVE-YYYY-XXXXX.md`
- Print: "✅ CVE-YYYY-XXXXX not present in [repo]. VEX justification added to [JIRA-KEY]: [justification]"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Output/reporting docs need alignment with new VEX artifacts.

You now define vex-justified-*/vex-needs-human-review-*, but output sections still describe the old “already-fixed” flow as the primary non-PR artifact path. Please align artifact documentation to avoid operator confusion.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@workflows/cve-fixer/.claude/commands/cve.fix.md` around lines 439 - 443,
Update the output/reporting docs to reference the new VEX artifact names and
flows instead of the old "already-fixed" wording: replace references to
`artifacts/cve-fixer/fixes/vex-justified-CVE-YYYY-XXXXX.md` and any mentions of
the old “already-fixed” flow with the corresponding new artifact names
(`vex-justified-*`, `vex-needs-human-review-*`) and update the printed/report
messages (e.g., the "✅ CVE-YYYY-XXXXX not present..." line) and the bullet list
for the auto-determined justification cases to explicitly document writing the
correct VEX artifact and leaving closure to a human reviewer.

When a vulnerable package is not found in application manifests
(requirements.txt, go.mod, package.json), check if it comes from
the container base image (Dockerfile.konflux FROM line):

- Use skopeo list-tags to check for newer base image tags
- If newer tag available: update FROM line → real PR with note to
  verify the fix is included in the new tag before merging
- If no newer tag: add Jira comment asking base image team (e.g. AIPCC
  for quay.io/aipcc/*) to release updated image → no PR created
- If no Dockerfile found: fall back to transitive dependency guidance

This covers both the general "base image CVE" case and the AIPCC-specific
scenario with a single generic mechanism — no AIPCC-specific handling needed.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

♻️ Duplicate comments (3)
workflows/cve-fixer/.claude/commands/cve.fix.md (3)

153-160: ⚠️ Potential issue | 🟠 Major

TARGET_BRANCHES still needs deduplication to prevent duplicate PR attempts.

If DEFAULT_BRANCH appears in ACTIVE_RELEASE_BRANCHES (e.g., both are "main"), the array will contain duplicates, causing redundant PR creation or push conflicts.

🛡️ Proposed fix to deduplicate TARGET_BRANCHES
   if [ "$REPO_TYPE" = "downstream" ]; then
     TARGET_BRANCHES=("$DEFAULT_BRANCH" "${ACTIVE_RELEASE_BRANCHES[@]}")
   else
     TARGET_BRANCHES=("$DEFAULT_BRANCH")
   fi
+
+  # Deduplicate while preserving order
+  UNIQUE_TARGET_BRANCHES=()
+  for b in "${TARGET_BRANCHES[@]}"; do
+    [[ -z "$b" ]] && continue
+    [[ " ${UNIQUE_TARGET_BRANCHES[*]} " == *" $b "* ]] || UNIQUE_TARGET_BRANCHES+=("$b")
+  done
+  TARGET_BRANCHES=("${UNIQUE_TARGET_BRANCHES[@]}")
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@workflows/cve-fixer/.claude/commands/cve.fix.md` around lines 153 - 160,
TARGET_BRANCHES can contain duplicates when DEFAULT_BRANCH also appears in
ACTIVE_RELEASE_BRANCHES; update the assignment logic for TARGET_BRANCHES (the
variable) to deduplicate entries after assembling from DEFAULT_BRANCH and
ACTIVE_RELEASE_BRANCHES: build the initial list as currently done, then iterate
and push each branch into a new deduped array only if not already present (use a
shell associative map or membership check) so TARGET_BRANCHES ends up with
unique branch names.

628-628: ⚠️ Potential issue | 🟠 Major

Branch naming rule still omits target branch, contradicting multi-branch examples.

Line 177 shows fix/cve-YYYY-XXXXX-<package>-rhoai-3.4-attempt-1 (includes target branch), but this canonical rule omits it: fix/cve-YYYY-XXXXX-<package-name>-attempt-1. When processing multiple target branches, this creates name collisions.

📝 Proposed fix
-  - Use consistent naming: `fix/cve-YYYY-XXXXX-<package-name>-attempt-1`
+  - Use consistent naming: `fix/cve-YYYY-XXXXX-<package-name>-<target-branch>-attempt-1`
+    - Example: `fix/cve-2025-12345-urllib3-main-attempt-1`
+    - Example: `fix/cve-2025-12345-urllib3-rhoai-3.4-attempt-1`
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@workflows/cve-fixer/.claude/commands/cve.fix.md` at line 628, Update the
canonical branch-naming rule to include the target branch to avoid collisions:
change the rule string from "fix/cve-YYYY-XXXXX-<package-name>-attempt-1" to a
pattern that includes the target branch (for example matching the existing
example "fix/cve-YYYY-XXXXX-<package>-rhoai-3.4-attempt-1"); ensure any
validation or generation logic that uses the symbol/name pattern (branch naming
rule, branch creation logic) is updated to accept and produce the target-branch
token so multiple target branches produce distinct branch names.

163-172: ⚠️ Potential issue | 🟡 Minor

Fenced code block still missing language tag and blank lines.

The past review marked this as addressed in commit 177e6d0, but the fenced block at line 163 still lacks a language identifier and blank-line framing (MD040/MD031).

📝 Proposed fix
   
+   
-   ```
+   ```text
    upstream  llm-d/llm-d-inference-scheduler          → PR against: main
    midstream opendatahub-io/llm-d-inference-scheduler  → PR against: main
    downstream red-hat-data-services/llm-d-inference-scheduler → PRs against:
      - main
      - rhoai-3.3
      - rhoai-3.4
      - rhoai-3.4-ea.1
      - rhoai-3.4-ea.2
    ```
+
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@workflows/cve-fixer/.claude/commands/cve.fix.md` around lines 163 - 172, The
fenced code block containing the lines starting with "upstream 
llm-d/llm-d-inference-scheduler" is missing a language tag and surrounding blank
lines; update the block to use a language identifier (e.g., ```text) and ensure
there's a blank line before the opening ```text and a blank line after the
closing ``` so the block complies with MD040/MD031 rules.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@workflows/cve-fixer/.claude/commands/cve.fix.md`:
- Around line 511-516: The curl payload is built by directly interpolating
${COMMENT}, creating unsafe JSON; instead generate a properly escaped JSON body
(for example using jq or another JSON encoder) and pipe it into curl so you
don't interpolate ${COMMENT} into the -d string. Replace the inline -d
"{\"body\": \"${COMMENT}\"}" usage in the block that constructs AUTH and calls
curl (refer to variables AUTH, COMMENT, JIRA_BASE_URL, JIRA_KEY) with a safe
encoder step such as: create the JSON via jq -n --arg body "$COMMENT"
'{body:$body}' (or equivalent), then pass it to curl with --data `@-` (or -d `@-`)
so the comment is JSON-escaped and newline/quote characters cannot break the
payload.
- Around line 423-424: The pipeline currently assigns AVAILABLE_TAGS from the
skopeo command without checking failure; capture skopeo's exit status (or run it
in a subshell storing both output and exit code) when populating AVAILABLE_TAGS,
and if skopeo fails (non-zero exit) log a clear error and fail/exit the job (or
set a distinct error flag) instead of continuing with an empty AVAILABLE_TAGS;
update the code that references AVAILABLE_TAGS to rely on this failure handling
so you don't treat an empty list from a failed skopeo call as "no newer tags
available."
- Around line 246-258: Split the combined example into two clearly labeled
scenarios so operators don't confuse repositories and steps: update the block in
workflows/cve-fixer/.claude/commands/cve.fix.md to (1) label and describe
"Scenario 1: User has write access" (or "Push directly to origin") with the
/tmp/red-hat-data-services/llm-d-inference-scheduler clone and its five PR
targets (main, rhoai-3.3, rhoai-3.4, rhoai-3.4-ea.1, rhoai-3.4-ea.2), and (2)
label and describe "Scenario 2: User lacks write access (upstream repo)" (or
"Push to fork") with the /tmp/llm-d/llm-d-inference-scheduler clone and the
single fork-based PR example (head: <user>/llm-d-inference-scheduler:fix/... →
base: main); ensure each scenario is wrapped in its own fenced bash block and
update the inline comments to "Push directly to origin, create 5 PRs" and "Push
to fork, create 1 PR" respectively.
- Around line 483-485: The script calls an undefined helper version_is_safe when
evaluating PACKAGE_VERSION against CVE_AFFECTED_RANGE which will crash at
runtime; add a version_is_safe function (or replace the call) that accepts
(pkg_version, affected_range) and returns success (0) when the package version
is not in the affected range and failure (non‑zero) when it is, then keep the
existing logic that sets VEX_JUSTIFICATION="Vulnerable Code not Present" and
VEX_EVIDENCE using PACKAGE, PACKAGE_VERSION and CVE_AFFECTED_RANGE; reference
the symbol version_is_safe and ensure it is available before the conditional
that uses PACKAGE_VERSION and CVE_AFFECTED_RANGE so the branch executes
correctly.
- Around line 427-428: The awk comparison '$0 > cur' used to compute NEWER_TAGS
from AVAILABLE_TAGS compares strings lexicographically and breaks for semantic
versions; replace that step with a version-aware filter: sort AVAILABLE_TAGS
with sort -V and then select only entries that are semantically greater than
CURRENT_TAG (or run a short Python snippet that uses packaging.version.parse or
similar to compare each tag to CURRENT_TAG) so NEWER_TAGS contains only tags
with higher semantic versions; update the code that references NEWER_TAGS,
AVAILABLE_TAGS and CURRENT_TAG accordingly.
- Around line 488-490: The existing conditional that uses SCAN_OUTPUT and
PACKAGE in the elif branch wrongly checks for "No vulnerabilities found" and
will never match the intended scenario; update the conditional in the
conditional block that sets VEX_JUSTIFICATION/VEX_EVIDENCE to instead detect a
govulncheck informational vulnerability with no call stacks by using: check that
SCAN_OUTPUT contains "Vulnerability #" and the "${PACKAGE}" string and that it
does NOT contain "Call stacks" (e.g., replace the elif with: elif echo
"$SCAN_OUTPUT" | grep -q "Vulnerability #" && echo "$SCAN_OUTPUT" | grep -q
"${PACKAGE}" && ! echo "$SCAN_OUTPUT" | grep -q "Call stacks"; then), or
alternatively change the scan invocation to emit JSON (-json) and parse the JSON
output for findings without call traces for more robust detection.

---

Duplicate comments:
In `@workflows/cve-fixer/.claude/commands/cve.fix.md`:
- Around line 153-160: TARGET_BRANCHES can contain duplicates when
DEFAULT_BRANCH also appears in ACTIVE_RELEASE_BRANCHES; update the assignment
logic for TARGET_BRANCHES (the variable) to deduplicate entries after assembling
from DEFAULT_BRANCH and ACTIVE_RELEASE_BRANCHES: build the initial list as
currently done, then iterate and push each branch into a new deduped array only
if not already present (use a shell associative map or membership check) so
TARGET_BRANCHES ends up with unique branch names.
- Line 628: Update the canonical branch-naming rule to include the target branch
to avoid collisions: change the rule string from
"fix/cve-YYYY-XXXXX-<package-name>-attempt-1" to a pattern that includes the
target branch (for example matching the existing example
"fix/cve-YYYY-XXXXX-<package>-rhoai-3.4-attempt-1"); ensure any validation or
generation logic that uses the symbol/name pattern (branch naming rule, branch
creation logic) is updated to accept and produce the target-branch token so
multiple target branches produce distinct branch names.
- Around line 163-172: The fenced code block containing the lines starting with
"upstream  llm-d/llm-d-inference-scheduler" is missing a language tag and
surrounding blank lines; update the block to use a language identifier (e.g.,
```text) and ensure there's a blank line before the opening ```text and a blank
line after the closing ``` so the block complies with MD040/MD031 rules.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 6d3979fd-07c5-4300-9082-c28e67feab4e

📥 Commits

Reviewing files that changed from the base of the PR and between 841df52 and 60d6f70.

📒 Files selected for processing (1)
  • workflows/cve-fixer/.claude/commands/cve.fix.md

Comment on lines +246 to +258
```bash
# Clone once: /tmp/red-hat-data-services/llm-d-inference-scheduler
# User has write access → push directly, 5 PRs:
# PR 1 → base: main
# PR 2 → base: rhoai-3.3
# PR 3 → base: rhoai-3.4
# PR 4 → base: rhoai-3.4-ea.1
# PR 5 → base: rhoai-3.4-ea.2

# Clone once: /tmp/llm-d/llm-d-inference-scheduler
# User has NO write access → fork + 1 PR:
# PR 1 → head: <user>/llm-d-inference-scheduler:fix/... → base: main
```
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Example conflates two separate scenarios, potentially causing confusion.

The comment shows both write-access and fork scenarios for different repos, but the inline formatting suggests they're sequential steps for the same repo. This may confuse operators.

✏️ Proposed clarification
-   **Example for downstream with 4 active branches:**
+   **Examples:**
+   
+   **Scenario 1: User has write access (downstream repo)**
    ```bash
    # Clone once: /tmp/red-hat-data-services/llm-d-inference-scheduler
-   # User has write access → push directly, 5 PRs:
+   # Push directly to origin, create 5 PRs:
    # PR 1 → base: main
    # PR 2 → base: rhoai-3.3
    # PR 3 → base: rhoai-3.4
    # PR 4 → base: rhoai-3.4-ea.1
    # PR 5 → base: rhoai-3.4-ea.2
+   ```
 
+   **Scenario 2: User lacks write access (upstream repo)**
+   ```bash
    # Clone once: /tmp/llm-d/llm-d-inference-scheduler
-   # User has NO write access → fork + 1 PR:
+   # Push to fork, create 1 PR:
    # PR 1 → head: <user>/llm-d-inference-scheduler:fix/... → base: main
    ```
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)

[warning] 246-246: Fenced code blocks should be surrounded by blank lines

(MD031, blanks-around-fences)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@workflows/cve-fixer/.claude/commands/cve.fix.md` around lines 246 - 258,
Split the combined example into two clearly labeled scenarios so operators don't
confuse repositories and steps: update the block in
workflows/cve-fixer/.claude/commands/cve.fix.md to (1) label and describe
"Scenario 1: User has write access" (or "Push directly to origin") with the
/tmp/red-hat-data-services/llm-d-inference-scheduler clone and its five PR
targets (main, rhoai-3.3, rhoai-3.4, rhoai-3.4-ea.1, rhoai-3.4-ea.2), and (2)
label and describe "Scenario 2: User lacks write access (upstream repo)" (or
"Push to fork") with the /tmp/llm-d/llm-d-inference-scheduler clone and the
single fork-based PR example (head: <user>/llm-d-inference-scheduler:fix/... →
base: main); ensure each scenario is wrapped in its own fenced bash block and
update the inline comments to "Push directly to origin, create 5 PRs" and "Push
to fork, create 1 PR" respectively.

Comment on lines +423 to +424
AVAILABLE_TAGS=$(skopeo list-tags "docker://${IMAGE_REF}" 2>/dev/null | \
jq -r '.Tags[]' | sort -V)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Missing error handling for skopeo list-tags failures.

If skopeo fails (network issues, registry auth, image doesn't exist), the pipeline continues with empty AVAILABLE_TAGS, potentially causing incorrect "no newer tags available" conclusions.

🛡️ Proposed fix to handle skopeo failures
     echo "Checking for newer tags of: $IMAGE_REF (current: $CURRENT_TAG)"
 
     # List available tags (works for quay.io, registry.access.redhat.com, etc.)
-    AVAILABLE_TAGS=$(skopeo list-tags "docker://${IMAGE_REF}" 2>/dev/null | \
-      jq -r '.Tags[]' | sort -V)
+    if ! AVAILABLE_TAGS=$(skopeo list-tags "docker://${IMAGE_REF}" 2>&1 | jq -r '.Tags[]' 2>/dev/null | sort -V); then
+      echo "⚠️  Failed to list tags for ${IMAGE_REF}: ${AVAILABLE_TAGS}"
+      echo "Skipping base image check for this image (network/auth issue?)"
+      continue
+    fi
+    
+    if [ -z "$AVAILABLE_TAGS" ]; then
+      echo "⚠️  No tags found for ${IMAGE_REF} - may require authentication or image doesn't exist"
+      continue
+    fi
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@workflows/cve-fixer/.claude/commands/cve.fix.md` around lines 423 - 424, The
pipeline currently assigns AVAILABLE_TAGS from the skopeo command without
checking failure; capture skopeo's exit status (or run it in a subshell storing
both output and exit code) when populating AVAILABLE_TAGS, and if skopeo fails
(non-zero exit) log a clear error and fail/exit the job (or set a distinct error
flag) instead of continuing with an empty AVAILABLE_TAGS; update the code that
references AVAILABLE_TAGS to rely on this failure handling so you don't treat an
empty list from a failed skopeo call as "no newer tags available."

Comment on lines +427 to +428
NEWER_TAGS=$(echo "$AVAILABLE_TAGS" | awk -v cur="$CURRENT_TAG" '$0 > cur')
done
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Lexicographic comparison will fail for semantic version tags.

The awk comparison '$0 > cur' performs lexicographic string comparison, which breaks for semantic versions (e.g., "1.2.10" < "1.2.3" lexicographically). Combined with sort -V on line 424, this produces incorrect results.

🔧 Proposed fix using sort -V for version comparison
     # List available tags (works for quay.io, registry.access.redhat.com, etc.)
     AVAILABLE_TAGS=$(skopeo list-tags "docker://${IMAGE_REF}" 2>/dev/null | \
       jq -r '.Tags[]' | sort -V)
 
     # Find tags newer than current
-    NEWER_TAGS=$(echo "$AVAILABLE_TAGS" | awk -v cur="$CURRENT_TAG" '$0 > cur')
+    # Use version-aware comparison: print all tags, then take those after current
+    NEWER_TAGS=$(echo "$AVAILABLE_TAGS" | \
+      awk -v cur="$CURRENT_TAG" 'BEGIN{found=0} {if(found) print; if($0==cur) found=1}')

Alternatively, use a Python snippet for proper semantic version comparison:

-    NEWER_TAGS=$(echo "$AVAILABLE_TAGS" | awk -v cur="$CURRENT_TAG" '$0 > cur')
+    NEWER_TAGS=$(python3 -c "
+import sys
+from packaging import version
+tags = '''$AVAILABLE_TAGS'''.strip().split('\n')
+cur = '$CURRENT_TAG'
+for t in tags:
+    try:
+        if version.parse(t) > version.parse(cur):
+            print(t)
+    except:
+        if t > cur:  # fallback to lexicographic for non-semver tags
+            print(t)
+")
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@workflows/cve-fixer/.claude/commands/cve.fix.md` around lines 427 - 428, The
awk comparison '$0 > cur' used to compute NEWER_TAGS from AVAILABLE_TAGS
compares strings lexicographically and breaks for semantic versions; replace
that step with a version-aware filter: sort AVAILABLE_TAGS with sort -V and then
select only entries that are semantically greater than CURRENT_TAG (or run a
short Python snippet that uses packaging.version.parse or similar to compare
each tag to CURRENT_TAG) so NEWER_TAGS contains only tags with higher semantic
versions; update the code that references NEWER_TAGS, AVAILABLE_TAGS and
CURRENT_TAG accordingly.

Comment on lines +483 to +485
elif [ -n "$PACKAGE_VERSION" ] && version_is_safe "$PACKAGE_VERSION" "$CVE_AFFECTED_RANGE"; then
VEX_JUSTIFICATION="Vulnerable Code not Present"
VEX_EVIDENCE="Package '${PACKAGE}' present at version ${PACKAGE_VERSION} which is not in the affected range"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Undefined function version_is_safe referenced.

The code calls version_is_safe "$PACKAGE_VERSION" "$CVE_AFFECTED_RANGE", but this function is never defined in the documentation. This will cause a runtime error.

🔧 Proposed fix: Define the function or provide alternative

Option 1: Define the function (add before line 473):

# Compare package version against CVE affected range
# Returns 0 (true) if version is safe, 1 (false) if vulnerable
version_is_safe() {
  local pkg_version="$1"
  local affected_range="$2"
  
  # This is a placeholder - actual implementation depends on version format
  # For Python/Node: use pip or npm to compare versions
  # For Go: use go list -m to compare module versions
  
  # Example for Python using pip:
  python3 -c "
from packaging import version, specifiers
pkg_ver = version.parse('${pkg_version}')
spec = specifiers.SpecifierSet('${affected_range}')
exit(0 if pkg_ver not in spec else 1)
" 2>/dev/null
  return $?
}

Option 2: Use external tools directly (replace lines 483-485):

-  elif [ -n "$PACKAGE_VERSION" ] && version_is_safe "$PACKAGE_VERSION" "$CVE_AFFECTED_RANGE"; then
-    VEX_JUSTIFICATION="Vulnerable Code not Present"
-    VEX_EVIDENCE="Package '${PACKAGE}' present at version ${PACKAGE_VERSION} which is not in the affected range"
+  elif [ -n "$PACKAGE_VERSION" ]; then
+    # Check version against CVE database using grype or similar
+    if ! grype "${PACKAGE}:${PACKAGE_VERSION}" 2>/dev/null | grep -q "$CVE_ID"; then
+      VEX_JUSTIFICATION="Vulnerable Code not Present"
+      VEX_EVIDENCE="Package '${PACKAGE}' present at version ${PACKAGE_VERSION} which is not in the affected range"
+    fi
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@workflows/cve-fixer/.claude/commands/cve.fix.md` around lines 483 - 485, The
script calls an undefined helper version_is_safe when evaluating PACKAGE_VERSION
against CVE_AFFECTED_RANGE which will crash at runtime; add a version_is_safe
function (or replace the call) that accepts (pkg_version, affected_range) and
returns success (0) when the package version is not in the affected range and
failure (non‑zero) when it is, then keep the existing logic that sets
VEX_JUSTIFICATION="Vulnerable Code not Present" and VEX_EVIDENCE using PACKAGE,
PACKAGE_VERSION and CVE_AFFECTED_RANGE; reference the symbol version_is_safe and
ensure it is available before the conditional that uses PACKAGE_VERSION and
CVE_AFFECTED_RANGE so the branch executes correctly.

Comment on lines +488 to +490
elif echo "$SCAN_OUTPUT" | grep -q "No vulnerabilities found" && echo "$SCAN_OUTPUT" | grep -q "${PACKAGE}"; then
VEX_JUSTIFICATION="Vulnerable Code not in Execute Path"
VEX_EVIDENCE="govulncheck found module ${PACKAGE} in dependency tree but confirmed vulnerable symbol is not called in code path"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check govulncheck output format for the "module present but not in call path" case
# This helps determine the correct pattern to match

echo "Searching for examples of govulncheck output patterns in existing repos..."
rg -n -A5 -B5 "govulncheck.*verbose" -g '*.md' -g '*.sh'

Repository: ambient-code/workflows

Length of output: 140


🏁 Script executed:

# Read the file to see the actual code at the specified lines
cat -n workflows/cve-fixer/.claude/commands/cve.fix.md | head -510 | tail -40

Repository: ambient-code/workflows

Length of output: 2493


🏁 Script executed:

# Also check the broader context around this code to understand the logic flow
cat -n workflows/cve-fixer/.claude/commands/cve.fix.md | head -520 | tail -100

Repository: ambient-code/workflows

Length of output: 6625


🏁 Script executed:

# Search for govulncheck documentation or references in the repository
rg -n "govulncheck" workflows/cve-fixer/ -A3 -B3

Repository: ambient-code/workflows

Length of output: 506


🌐 Web query:

govulncheck output format "No vulnerabilities found" package in dependency tree

💡 Result:

govulncheck does not emit a per-package “No vulnerabilities found” entry for packages in your dependency tree. In the default text output, “No vulnerabilities found.” is simply the overall result when the scan produces zero findings. [1]

If you need a machine-checkable way to detect “no vulns”, use JSON output and check whether the stream contains any finding messages:

  • The JSON output is a stream of messages (config, optional progress, optional osv, and finding). [2]
  • A vulnerability only “counts” as detected when you get one or more finding messages. Finding includes a trace (frames with module/package/function/position info) when applicable. [2]

If your goal is “why is module/package X in my build graph?”, govulncheck won’t print a dependency tree for non-vulnerable packages—use go mod why <module> / go mod graph for that (and then correlate with any finding.trace when there are vulnerabilities). [1][2]

Sources
[1] govulncheck command documentation (Go Packages) (pkg.go.dev)
[2] internal/govulncheck JSON protocol structs (Message/Finding/Trace) (github.com)

Citations:


🌐 Web query:

govulncheck call graph vulnerable symbol not called output example

💡 Result:

Here’s what govulncheck looks like when a dependency contains a vulnerability, but your code does not call any vulnerable symbol (so no reachable call graph / call stacks are shown). In the Go team’s tutorial, it appears under an “Informational” section:

=== Informational ===

Found 1 vulnerability in packages that you import, but there are no call
stacks leading to the use of this vulnerability. You may not need to
take any action. See https://pkg.go.dev/golang.org/x/vuln/cmd/govulncheck
for details.

Vulnerability `#1`: GO-2022-1059
  ...
  Found in: golang.org/x/text@v0.3.5
  Fixed in: golang.org/x/text@v0.3.8

[1]

For contrast, when govulncheck can build a reachable call stack (call graph evidence) to a vulnerable symbol, it prints a “Call stacks in your code:” section, e.g.:

Call stacks in your code:
  main.go:12:29: vuln.tutorial.main calls golang.org/x/text/language.Parse

[1][2]

And a longer, multi-step call chain can be summarized like:

... calls <pkg A>, which eventually calls <vulnerable symbol>

[3]

If you want more detailed call stacks when they exist, govulncheck supports -show traces. [2]

Sources:
[1] Go tutorial output showing “Informational” when no call stacks lead to vulnerable use
[2] govulncheck docs (pkg.go.dev) describing call stack summaries and -show traces
[3] Example govulncheck output with “which eventually calls …” call-chain summary


🏁 Script executed:

# Check if there are any test files or examples in the repository that might show govulncheck usage
fd -t f "test" workflows/cve-fixer/ | head -20

Repository: ambient-code/workflows

Length of output: 48


🏁 Script executed:

# Search for any comments or documentation about govulncheck behavior in the repo
rg -n "govulncheck" workflows/cve-fixer/ --type md --type sh -B5 -A5

Repository: ambient-code/workflows

Length of output: 839


The govulncheck condition check is logically flawed and will never match the intended scenario.

The condition checks for both "No vulnerabilities found" AND the package being present. However, govulncheck only prints "No vulnerabilities found" when the scan produces zero findings overall. When a module exists in the dependency tree but its vulnerable code is not reachable (the intended scenario), govulncheck shows an "Informational" section with "Vulnerability #" entries—not "No vulnerabilities found."

To correctly detect when a package is vulnerable but the vulnerable code is not in the call path, check for:

  • "Vulnerability #" present (confirms vulnerability found)
  • Package name present
  • "Call stacks" absent (confirms vulnerable code not reachable)

Update the condition to:

elif echo "$SCAN_OUTPUT" | grep -q "Vulnerability #" && echo "$SCAN_OUTPUT" | grep -q "${PACKAGE}" && ! echo "$SCAN_OUTPUT" | grep -q "Call stacks"; then

Alternatively, parse govulncheck's JSON output (-json flag) for more reliable detection of finding messages without call traces.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@workflows/cve-fixer/.claude/commands/cve.fix.md` around lines 488 - 490, The
existing conditional that uses SCAN_OUTPUT and PACKAGE in the elif branch
wrongly checks for "No vulnerabilities found" and will never match the intended
scenario; update the conditional in the conditional block that sets
VEX_JUSTIFICATION/VEX_EVIDENCE to instead detect a govulncheck informational
vulnerability with no call stacks by using: check that SCAN_OUTPUT contains
"Vulnerability #" and the "${PACKAGE}" string and that it does NOT contain "Call
stacks" (e.g., replace the elif with: elif echo "$SCAN_OUTPUT" | grep -q
"Vulnerability #" && echo "$SCAN_OUTPUT" | grep -q "${PACKAGE}" && ! echo
"$SCAN_OUTPUT" | grep -q "Call stacks"; then), or alternatively change the scan
invocation to emit JSON (-json) and parse the JSON output for findings without
call traces for more robust detection.

Comment on lines +511 to +516
AUTH=$(echo -n "${JIRA_EMAIL}:${JIRA_API_TOKEN}" | base64)
curl -s -X POST \
-H "Authorization: Basic ${AUTH}" \
-H "Content-Type: application/json" \
-d "{\"body\": \"${COMMENT}\"}" \
"${JIRA_BASE_URL}/rest/api/3/issue/${JIRA_KEY}/comment"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Unsafe JSON encoding in curl payload creates injection risk.

The code directly interpolates ${COMMENT} into a JSON string without escaping. This will break if COMMENT contains quotes, newlines, or other special characters, and could enable injection attacks.

🔒 Proposed fix using jq for safe JSON encoding
   # Post comment via Jira API
   AUTH=$(echo -n "${JIRA_EMAIL}:${JIRA_API_TOKEN}" | base64)
-  curl -s -X POST \
-    -H "Authorization: Basic ${AUTH}" \
-    -H "Content-Type: application/json" \
-    -d "{\"body\": \"${COMMENT}\"}" \
-    "${JIRA_BASE_URL}/rest/api/3/issue/${JIRA_KEY}/comment"
+  
+  # Use jq to safely construct JSON payload
+  PAYLOAD=$(jq -n \
+    --arg body "$COMMENT" \
+    '{body: $body}')
+  
+  curl -s -X POST \
+    -H "Authorization: Basic ${AUTH}" \
+    -H "Content-Type: application/json" \
+    -d "$PAYLOAD" \
+    "${JIRA_BASE_URL}/rest/api/3/issue/${JIRA_KEY}/comment"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
AUTH=$(echo -n "${JIRA_EMAIL}:${JIRA_API_TOKEN}" | base64)
curl -s -X POST \
-H "Authorization: Basic ${AUTH}" \
-H "Content-Type: application/json" \
-d "{\"body\": \"${COMMENT}\"}" \
"${JIRA_BASE_URL}/rest/api/3/issue/${JIRA_KEY}/comment"
AUTH=$(echo -n "${JIRA_EMAIL}:${JIRA_API_TOKEN}" | base64)
# Use jq to safely construct JSON payload
PAYLOAD=$(jq -n \
--arg body "$COMMENT" \
'{body: $body}')
curl -s -X POST \
-H "Authorization: Basic ${AUTH}" \
-H "Content-Type: application/json" \
-d "$PAYLOAD" \
"${JIRA_BASE_URL}/rest/api/3/issue/${JIRA_KEY}/comment"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@workflows/cve-fixer/.claude/commands/cve.fix.md` around lines 511 - 516, The
curl payload is built by directly interpolating ${COMMENT}, creating unsafe
JSON; instead generate a properly escaped JSON body (for example using jq or
another JSON encoder) and pipe it into curl so you don't interpolate ${COMMENT}
into the -d string. Replace the inline -d "{\"body\": \"${COMMENT}\"}" usage in
the block that constructs AUTH and calls curl (refer to variables AUTH, COMMENT,
JIRA_BASE_URL, JIRA_KEY) with a safe encoder step such as: create the JSON via
jq -n --arg body "$COMMENT" '{body:$body}' (or equivalent), then pass it to curl
with --data `@-` (or -d `@-`) so the comment is JSON-escaped and newline/quote
characters cannot break the payload.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant